Target .NET 8, rewrite webhook controller #113
Conversation
| private readonly IDeliveryCacheManager _cacheManager; | ||
|
|
||
| public WebhooksController(IDeliveryCacheManager cacheManager) | ||
| { |
There was a problem hiding this comment.
I would keep the original constructor, rather then using this hybrid solution for field validation. but it is up to your decision.
There was a problem hiding this comment.
you might be right, especially since rest of the project doesn't use primary constructors either. reverting.
|
|
||
| [HttpPost] | ||
| public async Task<IActionResult> Index([FromBody] DeliveryWebhookModel model) | ||
| public async Task<IActionResult> Index([FromBody] WebhookNotification? webhook) |
There was a problem hiding this comment.
shouldn't we support both until the 1.11.? or is it ok to cause this breaking now?
There was a problem hiding this comment.
the underlying package is https://github.com/kontent-ai/aspnetcore-extensions and it still supports legacy webhooks as well. the boilerplate should ideally use the most up-to-date approach and since we'll be removing legacy webhooks soon, I don't see a problem with adopting the new webhooks (wouldn't want users to scaffold a new project with webhooks that won't be a thing in a month).
| dependencies.Add(CacheHelpers.GetItemDependencyKey(item.Codename)); | ||
| } | ||
|
|
||
| dependencies.Add(CacheHelpers.GetItemsDependencyKey()); |
There was a problem hiding this comment.
What about items dependencyKey?
also when there is taxonomy update, there were other dependecies as well.
Current webhooks do not include info about touched dependencies though.
There was a problem hiding this comment.
yes, this was way too simple. I added more complex logic with pattern matching to wipe the keys of (potentially) dependent entities
| return Ok(); | ||
| } | ||
|
|
||
| private static string? GetDependencyKey(WebhookModel notification) => |
There was a problem hiding this comment.
Is this same for every webhook model?
Motivation
Fixes #111 #112
Checklist
How to test
If manual testing is required, what are the steps?